Skip to content

Conversation

@ch1seL
Copy link

@ch1seL ch1seL commented Nov 3, 2023

When using overload ForwardedPortRemote without boundHost parameter it is still transmitted as empty string and then resolved by DnsAbstraction.GetHostAddresses -> '::1'.

This is necessary to simulate ssh command like this ssh -R 80:127.0.0.1:5000 [email protected] (https://localhost.run/docs/)

@ch1seL ch1seL force-pushed the allow-to-omit-bound-host-address-paramter-forwardedportremote branch from 186aec3 to 092b486 Compare November 15, 2023 21:43
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR modifies ForwardedPortRemote to support optional BoundHost specification, allowing the bound host address to be null to simulate SSH remote port forwarding commands like ssh -R 80:127.0.0.1:5000 [email protected].

Key Changes:

  • Removed the null check for boundHostAddress parameter in the main constructor
  • Modified the constructor chain to pass null instead of empty string when boundHost is not specified
  • Updated the BoundHost property getter to handle null BoundHostAddress

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

/// <param name="port">The port.</param>
public ForwardedPortRemote(string boundHost, uint boundPort, string host, uint port)
: this(DnsAbstraction.GetHostAddresses(boundHost)[0],
: this(boundHost == null ? null : DnsAbstraction.GetHostAddresses(boundHost)[0],
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Direct array indexing without bounds checking could throw IndexOutOfRangeException if GetHostAddresses returns an empty array. Consider adding validation to ensure the array is not empty before accessing the first element.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant